Skip to content

Conversation

rogercoll
Copy link

Implements the WaitableChild struct for freebsd systems. In addition, the Wait approach will be used by default for freebsd systems.

The PR explicitly implements the WaitableChild only for freebsd, that is because of the lack of testing in other BSD systems. I would expect other systems supporting the kqueue event notification interface to work as well (NetBSD, OpenBSD, DragonFly BSD, and macOS). Would it make sense to extend the implementation to cover these OSes too?

Usage of smol-rs/async-io#207

Tests: all tests run with the WaitableChild instance.

Comment on lines 237 to 246
let exit_filter = unsafe { Filter::new(Exit::from_pid(NonZeroI32::new_unchecked(
child
.id()
.try_into()
.expect("could not transform pid to i32 type")
)))?
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you reformat this to match rusfmt formatting? Unfortunately rustfmt doesn't auto-format code in macro blocks.

@notgull
Copy link
Member

notgull commented Aug 3, 2025

@rogercoll I'm back, can you rebase on master?

@rogercoll rogercoll force-pushed the freebsd_kqueue_waitable_child branch from 5e7c2b5 to a803019 Compare August 4, 2025 09:37
@rogercoll
Copy link
Author

@rogercoll I'm back, can you rebase on master?

Done, not sure if the failure of macos CI is related to this PR 🤔

Thanks!

@rogercoll rogercoll requested a review from notgull August 4, 2025 09:42
@notgull
Copy link
Member

notgull commented Aug 7, 2025

It's a bug in async-io, I think. It should be fixed by updating to async-io v2.5 in this PR.

Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be done for all of the following platforms, right?

#[cfg(any(
    target_vendor = "apple",
    target_os = "freebsd",
    target_os = "netbsd",
    target_os = "openbsd",
    target_os = "dragonfly",
))]

@rogercoll
Copy link
Author

target_vendor = "apple",

That's right, added support in e2583f4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants